feat(x): enable autocli for txs in modules #24387
Conversation
github-actions
bot
added
C:x/staking
C:x/gov
C:x/slashing
C:x/distribution
julienrbrt
force-pushed
the
julien/autocli-tx
branch
from
c9958fb to
4ed54dd
Compare
github-actions
bot
added
C:Simulations
C:Store
C:x/evidence
C:Cosmovisor
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
github-actions
bot
removed
C:Simulations
C:Store
C:x/evidence
C:Cosmovisor
📝 WalkthroughWalkthroughThe changes update the CLI command interface across multiple modules. A new changelog entry documents a simplified command syntax for sending tokens via aliasing instead of using full addresses. In many modules, the AutoCLI options were modified to set the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant AutoCLI
Note over CLI,AutoCLI: Legacy command handlers are deprecated
User->>CLI: Enter transaction command (e.g., send tokens)
CLI->>AutoCLI: Check AutoCLIOptions (EnhanceCustomCommand=true)
AutoCLI-->>CLI: Generate enhanced command processing
CLI->>User: Execute simplified command using alias
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
CHANGELOG.md (1)
77-78: Well-documented feature in changelogThe changelog entry properly documents the addition of autocli commands for all proposals and the improved UX. The example showing how users can now use
<appd> tx bank send alice bob 100stakeinstead of the more complex form with key addresses is clear and matches the PR description exactly. This simplified syntax for commands using aliasing instead of addresses is a valuable UX improvement.There's a minor formatting issue with the indentation in the list item. The markdown linter suggests that line 78 should have 2 spaces of indentation instead of 4.
- * f.e `<appd> tx bank send alice $(simd keys show bob -a) 100stake` can be directly `<appd> tx bank send alice bob 100stake` now. This is true for all fields that take an address. + * f.e `<appd> tx bank send alice $(simd keys show bob -a) 100stake` can be directly `<appd> tx bank send alice bob 100stake` now. This is true for all fields that take an address.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
78-78: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
x/auth/vesting/client/cli/tx.go (2)
27-41: Document how to use the AutoCLI alternatives.While the code changes correctly remove the deprecated commands from
GetTxCmd, it would be helpful to add a comment or documentation reference guiding users on how to use the new AutoCLI-based commands that replace the deprecated ones. This would facilitate smoother transition for users of these commands.func GetTxCmd(ac address.Codec) *cobra.Command { txCmd := &cobra.Command{ Use: types.ModuleName, Short: "Vesting transaction subcommands", + Long: "Vesting transaction subcommands. Some commands previously available here have been deprecated in favor of AutoCLI alternatives.", DisableFlagParsing: true, SuggestionsMinimumDistance: 2, RunE: client.ValidateCmd, } txCmd.AddCommand( NewMsgCreatePeriodicVestingAccountCmd(ac), ) return txCmd }
144-146: Consider adding deprecation notice toNewMsgCreatePeriodicVestingAccountCmd.For consistency with the other command handlers in this file, consider whether
NewMsgCreatePeriodicVestingAccountCmdshould also be marked as deprecated if it's going to be replaced by an AutoCLI command in the future. If it's intentionally being kept (perhaps because there's no AutoCLI equivalent yet), a comment explaining why would be helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
CHANGELOG.md(1 hunks)x/auth/autocli.go(1 hunks)x/auth/vesting/autocli.go(1 hunks)x/auth/vesting/client/cli/tx.go(2 hunks)x/authz/client/cli/tx.go(1 hunks)x/authz/module/autocli.go(1 hunks)x/bank/client/cli/tx.go(1 hunks)x/circuit/autocli.go(1 hunks)x/consensus/autocli.go(1 hunks)x/crisis/autocli.go(1 hunks)x/distribution/autocli.go(1 hunks)x/distribution/client/cli/tx.go(4 hunks)x/feegrant/module/autocli.go(1 hunks)x/gov/autocli.go(1 hunks)x/gov/client/cli/tx.go(3 hunks)x/group/client/cli/tx.go(8 hunks)x/group/module/autocli.go(1 hunks)x/mint/autocli.go(1 hunks)x/nft/module/autocli.go(1 hunks)x/protocolpool/autocli.go(1 hunks)x/slashing/autocli.go(1 hunks)x/staking/autocli.go(1 hunks)x/staking/client/cli/tx.go(4 hunks)x/upgrade/autocli.go(1 hunks)x/upgrade/client/cli/tx.go(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
78-78: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: tests (03)
- GitHub Check: tests (02)
- GitHub Check: tests (01)
- GitHub Check: tests (00)
- GitHub Check: test-system-legacy
- GitHub Check: test-system
- GitHub Check: test-sim-nondeterminism
- GitHub Check: test-integration
- GitHub Check: test-e2e
- GitHub Check: build (amd64)
- GitHub Check: Gosec
- GitHub Check: golangci-lint
- GitHub Check: Summary
🔇 Additional comments (41)
x/authz/module/autocli.go (1)
51-51: LGTM: Enabled AutoCLI for transaction commandsSetting
EnhanceCustomCommandto true aligns with the PR objective of enabling autocli for transactions in modules, which will improve user experience by simplifying transaction commands.x/group/client/cli/tx.go (2)
191-192: LGTM: Proper deprecation of legacy command handlersThe deprecation messages are clear and consistent across all command handlers. This approach of marking commands as deprecated rather than removing them outright maintains backward compatibility while encouraging users to migrate to the new AutoCLI commands.
Also applies to: 237-238, 456-457, 538-539, 663-664, 711-712, 773-774, 806-807
29-50:❓ Verification inconclusive
Confirm that AutoCLI equivalents exist for all deprecated commands
I see that several group command handlers have been deprecated, but they've also been removed from the TxCmd function, which means they're no longer accessible via the CLI. Please verify that AutoCLI equivalents exist for all these commands to ensure users can still perform the same operations.
🏁 Script executed:
#!/bin/bash # Check if AutoCLI equivalents exist for deprecated commands by examining the module's autocli.go file echo "Checking for AutoCLI equivalents for deprecated group commands..." cat $(fd -t f autocli.go | grep -i group) | grep -B 5 -A 5 "EnhanceCustomCommand" echo "Listing available CLI commands for group module..." if command -v go &> /dev/null; then cd cmd/simd && go run . tx group --help 2>/dev/null || echo "Cannot run simd binary, manual verification needed" else echo "Go binary not found, manual verification needed" fiLength of output: 973
AutoCLI Equivalents Verification for Deprecated Group Commands
It appears that the AutoCLI configuration in
x/group/module/autocli.gois set to enable AutoCLI for at least one deprecated command (evidenced by theEnhanceCustomCommand: trueflag on theUpdateGroupAdminRPC). Although the automated check for CLI commands via the simd binary couldn’t run (likely due to the absence of thecmd/simddirectory), please verify manually that all deprecated command handlers have been replaced with proper AutoCLI equivalents to ensure users can still perform the same operations.
- Confirm the presence of
EnhanceCustomCommand: true(or similar AutoCLI configurations) for each deprecated command inx/group/module/autocli.go.- Manually check the CLI help output (or alternative binary) to ensure that the AutoCLI-based commands expose all necessary functionality.
x/nft/module/autocli.go (1)
85-86: LGTM: Enabled AutoCLI for NFT transaction commandsSetting
EnhanceCustomCommandto true for the NFT module's transaction commands aligns with the PR objective of enhancing user experience by enabling AutoCLI features.x/circuit/autocli.go (1)
36-37: LGTM: Enabled AutoCLI for circuit transaction commandsSetting
EnhanceCustomCommandto true for the circuit module's transaction commands aligns with the PR objective of enabling autocli for transactions across modules.x/crisis/autocli.go (1)
15-16: Good addition of enhanced custom commands supportThe addition of
EnhanceCustomCommand: trueenables the autocli feature for transactions in the crisis module, which aligns with the PR objective of enabling autocli for transactions in all modules. This change will improve user experience by allowing simplified transaction commands as mentioned in the PR description.x/mint/autocli.go (1)
35-36: Approve enabling autocli for mint moduleSetting
EnhanceCustomCommand: truefor the mint module enables improved command handling through the autocli system. This is consistent with the PR's objective to enable autocli for transactions across all modules, following extensive testing in v0.52. The enhancement will provide a better user experience with simplified commands.x/auth/vesting/autocli.go (1)
12-13: Approve enabling autocli for vesting transactionsThe change from previous false/deprecated configuration to
EnhanceCustomCommand: truecorrectly enables autocli for vesting transactions. According to the PR summary, related CLI command handlers have been deprecated in favor of using the autocli-generated commands. This transition provides a more consistent command interface while minimizing breaking changes since old commands remain available but deprecated.x/consensus/autocli.go (1)
30-31: Enhanced command processing enabled for consensus module transactions.This change enables the autocli feature for consensus module transactions by setting
EnhanceCustomCommand: true. This is consistent with the PR objectives to simplify transaction commands and improve user experience.x/group/module/autocli.go (1)
128-128: Enhanced command processing enabled for group module transactions.The change from
falsetotrueforEnhanceCustomCommandenables autocli for the group module transactions. This is aligned with the PR objectives to improve user experience by enabling auto-resolution of fields and simplifying transaction commands.x/protocolpool/autocli.go (1)
40-41: Enhanced command processing enabled for protocolpool module transactions.This change enables the autocli feature for protocolpool module transactions by setting
EnhanceCustomCommand: true. This is consistent with the PR objectives to simplify transaction commands and improve user experience.x/slashing/autocli.go (1)
40-41: Enhanced command processing enabled for slashing module transactions.This change enables the autocli feature for slashing module transactions by setting
EnhanceCustomCommand: true. This is consistent with the PR objectives to simplify transaction commands and improve user experience.x/feegrant/module/autocli.go (1)
49-51: Good enhancement enabling AutoCLI for feegrant transactionsSetting
EnhanceCustomCommand: trueenables the autocli feature for the transactions in the feegrant module, which improves the user experience by allowing automatic resolution of fields and simplified command syntax.x/bank/client/cli/tx.go (2)
21-36: Removal of deprecated send command is aligned with AutoCLI adoptionThe removal of
NewSendTxCmd(ac)from the command list properly implements the transition to AutoCLI-generated commands, which provide a better user experience as stated in the PR objectives.
38-41: Well-documented deprecation notice with clear reasoningThe deprecation notice provides clear information about why the command is deprecated and refers to the solution that will be implemented in the future. This helps maintainers understand the rationale behind the change.
x/authz/client/cli/tx.go (2)
38-54: Revoke command removal aligns with AutoCLI adoption strategyThe removal of
NewCmdRevokeAuthorization(ac)from the command list is consistent with the PR's approach to transition from manually defined commands to AutoCLI commands, improving UX.
230-232: Clear deprecation notice for the revoke authorization commandThe deprecation notice explicitly states that the command will be removed in a future release and explains that the module is transitioning to AutoCLI. This provides good context for future maintainers.
x/distribution/autocli.go (1)
90-93: Good enhancement enabling AutoCLI for distribution transactionsSetting
EnhanceCustomCommand: trueenables enhanced command processing for distribution transactions, allowing for automatic resolution of fields and simplified command syntax as described in the PR objectives.x/staking/autocli.go (1)
143-144: LGTM! Good enhancement for user experience.Adding
EnhanceCustomCommand: trueenables the autocli feature for transactions in the staking module, aligning with the PR's goal of improving user experience through simplified commands.x/auth/autocli.go (1)
80-81: Activation of autocli for auth module transactions approved.Setting
EnhanceCustomCommand: trueproperly enables the autocli functionality for the auth module, consistent with the PR's objective of enabling this feature across modules.x/upgrade/client/cli/tx.go (2)
135-136: Appropriate deprecation notice.The deprecation comment correctly indicates that this command will be replaced by its autocli equivalent, providing clear guidance to users about the transition.
32-43:❓ Verification inconclusive
Verify removal impact on existing commands.
The function
GetTxCmdappears to no longer include theNewCmdSubmitCancelUpgradeProposalcommand. Make sure this removal won't break existing workflows for users who haven't yet transitioned to autocli commands.
🏁 Script executed:
#!/bin/bash # Check if there are any test failures related to the cancel-software-upgrade command rg -A 2 -B 2 "cancel-software-upgrade" --glob "*.go" | grep -v "// Deprecated"Length of output: 688
Confirm Impact of Removing the Cancel Upgrade Command
- It appears that while the cancellation command (
NewCmdSubmitCancelUpgradeProposal) is still defined inx/upgrade/client/cli/tx.go(and referenced in related files likex/gov/client/cli/prompt.go), it is no longer added to the root tx command withinGetTxCmd.- Please verify that this removal is intentional and that users who haven’t transitioned to autocli commands are not adversely affected.
- Ensure that any tests or documentation referencing the cancel command are updated accordingly.
x/upgrade/autocli.go (1)
50-51: LGTM! Consistent enablement of autocli for the upgrade module.Enabling
EnhanceCustomCommandfor the upgrade module is consistent with the changes in other modules and supports the PR's objective of activating autocli for transactions.x/staking/client/cli/tx.go (5)
36-52: Command removals align with the transition to AutoCLIThe removal of the deprecated delegate, redelegate, unbond, and cancel-unbonding commands from
NewTxCmdis consistent with the PR objectives to streamline commands using AutoCLI while maintaining backward compatibility.
177-178: Proper deprecation notice for NewDelegateCmdThe deprecation comment clearly indicates that this command will be removed in a future release and the module is transitioning to using its AutoCLI command instead, which aligns with the PR objectives.
225-226: Proper deprecation notice for NewRedelegateCmdThe deprecation comment clearly indicates that this command will be removed in a future release and the module is transitioning to using its AutoCLI command instead.
278-279: Proper deprecation notice for NewUnbondCmdThe deprecation comment clearly indicates that this command will be removed in a future release and the module is transitioning to using its AutoCLI command instead.
328-329: Proper deprecation notice for NewCancelUnbondingDelegationThe deprecation comment clearly indicates that this command will be removed in a future release and the module is transitioning to using its AutoCLI command instead.
x/gov/autocli.go (1)
97-98: Enabling EnhanceCustomCommand is key to AutoCLI integrationSetting
EnhanceCustomCommand: trueis the critical change that enables the AutoCLI functionality for transactions in the gov module, which is essential for the simplified command syntax mentioned in the PR objectives.x/distribution/client/cli/tx.go (5)
31-45: Command removals align with the transition to AutoCLIThe
NewTxCmdfunction now only includesNewWithdrawAllRewardsCmd, removing all deprecated commands. This aligns with the PR objectives to transition to AutoCLI-generated commands while maintaining backward compatibility.
71-72: Proper deprecation notice for NewWithdrawRewardsCmdThe deprecation comment clearly indicates that this command will be removed in a future release and the module is transitioning to using its AutoCLI command instead.
186-187: Proper deprecation notice for NewSetWithdrawAddrCmdThe deprecation comment clearly indicates that this command will be removed in a future release and the module is transitioning to using its AutoCLI command instead.
227-228: Proper deprecation notice for NewFundCommunityPoolCmdThe deprecation comment clearly indicates that this command will be removed in a future release and the module is transitioning to using its AutoCLI command instead.
270-271: Proper deprecation notice for NewDepositValidatorRewardsPoolCmdThe deprecation comment clearly indicates that this command will be removed in a future release and the module is transitioning to using its AutoCLI command instead.
x/gov/client/cli/tx.go (4)
50-80: Command selection aligns with the transition to AutoCLIThe
NewTxCmdfunction now excludes the deprecated commands (deposit, vote, cancel-proposal) while keeping other commands that might not be fully supported by AutoCLI yet. This approach balances the transition to AutoCLI with maintaining necessary functionality.
155-156: Proper deprecation notice for NewCmdCancelProposalThe deprecation comment clearly indicates that this command will be removed in a future release and the module is transitioning to using its AutoCLI command instead.
256-257: Proper deprecation notice for NewCmdDepositThe deprecation comment clearly indicates that this command will be removed in a future release and the module is transitioning to using its AutoCLI command instead.
306-307: Proper deprecation notice for NewCmdVoteThe deprecation comment clearly indicates that this command will be removed in a future release and the module is transitioning to using its AutoCLI command instead.
x/auth/vesting/client/cli/tx.go (3)
43-46: Appropriate deprecation notice added.The deprecation notice is clear and informative, properly indicating that this command is deprecated and will be removed in a future release, directing users to the AutoCLI alternative.
94-96: Appropriate deprecation notice added.The deprecation notice for
NewMsgCreatePermanentLockedAccountCmdis consistent with the one forNewMsgCreateVestingAccountCmd, clearly indicating the command's status and future direction.
36-38:❓ Verification inconclusive
Check for consistency in command deprecation approach.
The
GetTxCmdfunction now includes onlyNewMsgCreatePeriodicVestingAccountCmdbut excludes the other two command handlers that have been marked as deprecated. However, there's an inconsistency as bothNewMsgCreateVestingAccountCmdandNewMsgCreatePermanentLockedAccountCmdare deprecated in favor of AutoCLI, whileNewMsgCreatePeriodicVestingAccountCmdremains unchanged without deprecation notice. This suggests that either this third command should also be deprecated for consistency, or there's a reason it hasn't been transitioned to AutoCLI yet.
🏁 Script executed:
#!/bin/bash # Check if there are any references to AutoCLI for periodic vesting accounts rg -A 3 "PeriodicVestingAccount.*AutoCLI" --type goLength of output: 53
Verify deprecation consistency for Periodic Vesting Account Command
The current implementation of
GetTxCmdonly registersNewMsgCreatePeriodicVestingAccountCmdwhile omittingNewMsgCreateVestingAccountCmdandNewMsgCreatePermanentLockedAccountCmd(both marked as deprecated in favor of AutoCLI). Our search for any AutoCLI references associated with the periodic vesting command returned no evidence—raising the question of whether this omission is intentional. Please verify whether:
- The periodic vesting command is meant to remain active without a deprecation notice, or
- It should be aligned with the other vesting commands by either being deprecated or migrated to AutoCLI.
Review the intended design/documentation and update the code or comments accordingly.
| @@ -47,7 +47,6 @@ func GetTxCmd(ac address.Codec) *cobra.Command { | |||
|
|
|||
| AuthorizationTxCmd.AddCommand( | |||
| NewCmdGrantAuthorization(ac), | |||
| NewCmdRevokeAuthorization(ac), | |||
There was a problem hiding this comment.
Should NewCmdExecAuthorization also be removed, since its in the autocli?
There was a problem hiding this comment.
Not yet because there's a small UX improvement in AutoCLI that changes the command UX. In the AutoCLI command you just need to pass the msg json while in the current command you were required to pass a tx JSON. So for 0.53 makes sense to not remove it yet. But afterwards it should be yeah
|
@technicallyty do we want this? |
i think its worthwhile, we should understand how it works tho |
Description
Enable autocli for txs in modules. client/v2 had tx support since v0.50 but it wasn't enabled for all modules.
We had enabled it in v0.52 after much testing because it gives a better UX.
The major UX improvement, next to be able to remove the tx CLI code from an app (like for queries), is key resolving of any fields.
This means, doing
$ simd tx bank send alice bob 100stakeinstead of
$ simd tx bank send alice $(simd keys show bob -a) 100stakeAnd here the previous bank send was a special command that was trying to resolve the first recipient.
Compared to v0.52 PRs like #17868, the previous commands aren't deleted from the SDK, but removed from the added command in cobra (because manually defined commands take precedence over autocli commands) and marked as deprecated.
This makes the PR way smaller as tests that needed to be rewritten, because depending on the CLI commands helpers, not need to (#17868 is again a good example).
Lastly, all commands can be eventually migrated and removed (that will clean-up a lot of code to maintain), even the ones not migrated here, as they only require minor UX changes.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
Refactor
Documentation